LSPS2: Fail (or abandon) intercepted HTLCs if LSP channel open fails #3712
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3712 +/- ##
==========================================
- Coverage 89.15% 89.15% -0.01%
==========================================
Files 156 156
Lines 123837 123922 +85
Branches 123837 123922 +85
==========================================
+ Hits 110408 110483 +75
+ Misses 10754 10748 -6
- Partials 2675 2691 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
martinsaposnic
force-pushed
the
handle-channel-open-failure
branch
from
f50496f to
0d8ba10
Compare
|
🔔 1st Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @tnull! This PR has been waiting for your review. |
| /// without resetting the state for a potential payment retry. It removes the intercept SCID | ||
| /// mapping along with any outbound channel state and related channel ID mappings associated with | ||
| /// the specified `user_channel_id`. | ||
| pub fn channel_open_abandoned( |
There was a problem hiding this comment.
I think to make this a bit safer, it should probably fail if we're already at PendingPaymentForward or later, i.e., we already opened the channel?
Also, what would happen if this is called after the LSPS2ServiceEvent::OpenChannel has been emitted?
There was a problem hiding this comment.
Now when abandon() is called, if it's not at PendingInitialPayment or PendingChannelOpen, then it fails.
Also, what would happen if this is called after the LSPS2ServiceEvent::OpenChannel has been emitted?
You mean if the channel is already opened, but our state machine hasn’t caught up yet and still thinks we're at PendingChannelOpen? (Is that possible?). In that case, if we call abandon(), we’d end up forgetting about a channel that was actually opened, potentially leaving funds locked. If that scenario is possible and we can't reliably detect it, then this version of abandon() isn't safe.
| })?; | ||
|
|
||
| let jit_channel = peer_state | ||
| .outbound_channels_by_intercept_scid |
| ), | ||
| })?; | ||
|
|
||
| jit_channel.state = match &jit_channel.state { |
There was a problem hiding this comment.
Maybe use the matches! macro here?
| @@ -24,6 +25,7 @@ use lightning_invoice::{Bolt11Invoice, InvoiceBuilder, RoutingFees}; | |||
| use bitcoin::hashes::{sha256, Hash}; | |||
| use bitcoin::secp256k1::{PublicKey, Secp256k1}; | |||
| use bitcoin::Network; | |||
| use lightning_types::payment::PaymentHash; | |||
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
Add two LSPS2Service methods: 'Abandoned' prunes all channel open state. 'Failed' resets JIT channel to fail HTLCs. It allows a retry on channel open. Closes lightningdevkit#3479.
…nt forwarding - Added a state check in channel_open_abandoned to ensure a channel cannot be abandoned after creation or payment forwarding, returning an error if attempted. - Refactored state handling to use matches! macro for clarity. - Addressed minor nits: fixed indentation and import ordering.
Add integration tests to verify channel open reset and pruning handlers. Tests cover: - channel_open_failed resetting state to allow retry. - channel_open_failed error on invalid state. - channel_open_abandoned pruning all open state. - error handling for nonexistent channels.
martinsaposnic
force-pushed
the
handle-channel-open-failure
branch
from
0d8ba10 to
5539995
Compare
|
🔔 1st Reminder Hey @tnull! This PR has been waiting for your review. |
Closes #3479
Allows the LSP to signal when a channel open fails or is abandoned.
PendingInitialPayment, allowing the payer to retry.Inspired by previous work and a suggestion from @tnull to add an abandon variant apart from the reset.